Update pairwise consistency test failures to support gracefully continiung #2201
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2201 +/- ##
==========================================
+ Coverage 79.05% 79.06% +0.01%
==========================================
Files 612 612
Lines 106222 106222
Branches 15008 15009 +1
==========================================
+ Hits 83973 83988 +15
+ Misses 21597 21583 -14
+ Partials 652 651 -1 ☔ View full report in Codecov by Sentry. |
| #else | ||
| return 0; | ||
| #endif | ||
| return 0; |
There was a problem hiding this comment.
Does this affect any unreachable-code analysis? As the return 0; could be seen as unreachable due to AWS_LC_FIPS_failure triggering an abort?
There was a problem hiding this comment.
In theory yes, but nothing seems to be catching the issue. Looks like GCC removed support for the check a while ago https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82100#c1, Clang has an option but we don't have it turned on https://clang.llvm.org/docs/DiagnosticsReference.html#wunreachable-code.
I enabled that option for clang and it correctly flags the exit(1) is unreachable after the abort. But it doesn't catch this new unreachable code. Reading the bug reports it looks like detecting unreachable code is a hard problem and very dependent on compiler versions and optimizations.
Debug build:
bcm.c:398:5: error: code will never be executed [-Werror,-Wunreachable-code]
398 | exit(1);
| ^~~~
Release build didn't find anything else with Clang.
andrewhop
force-pushed
the
pwct_refactor
branch
from
53c4de5 to
b8c17f6
Compare
andrewhop
force-pushed
the
pwct_refactor
branch
from
b8c17f6 to
c51d1fb
Compare
|
#2195 took care of the ML-KEM change, the lower function returns the error from the PCT and in the event that the AWS_LC_FIPS_failure returns it returns the failure result to the caller. |
Description of changes:
Currently AWS_LC_FIPS_failure always aborts the process and it is impossible to continue after calling that function. In a future change this will be optional and AWS-LC will not abort and the overall API call should return a failure. This change updates the current pairwise consistency test spots to theoretically return a failure.
Call-outs:
This PR does not change the behavior, if there is a PWCT failure the process still aborts.
Testing:
With the current AWS_LC_FIPS_failure implementation it is impossible to test this change.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.